fix(caddy): render args.Headers as one directive per line, quote values#300
Conversation
The `args.Headers` rendering path in the per-stack Caddyfile template had
two coupled bugs that combined to make `config.headers:` unusable from
stack yaml — surfaced when PAY-SPACE tried to ship the first multi-header
security baseline (PS-15/PS-20).
Bug 1 — same-line collapse:
The template line `header_down Server nginx ${addHeaders}` placed the
first user-supplied `header_down` on the SAME LINE as the existing
`Server nginx` directive, producing:
header_down Server nginx header_down X-Frame-Options DENY
header_down X-Content-Type-Options nosniff
…which Caddy rejects with "too many arguments" → Caddyfile fails to
load → deploy outage.
Bug 2 — unquoted values:
Values were rendered with `%s`, so any header with a space
(Content-Security-Policy, Permissions-Policy, Strict-Transport-Security
with multiple directives) tokenized as separate Caddyfile arguments
and crashed the parser.
Fix:
- Build the addHeaders string with a leading "\n " when non-empty so
every entry (including the first) starts on its own line.
- Use `%q` on the value so multi-token strings survive Caddy's
whitespace tokenizer. Single-token values (e.g. "DENY") get harmless
surrounding quotes — still valid Caddyfile.
- Sort entries by header name before emission so the rendered Caddyfile
is byte-stable across runs (Go map iteration is randomized; without
sorting, sc.CaddyfileEntry would flap and trigger spurious pulumi
diffs + Service-annotation change-hash churn).
Regression tests cover all three properties (own-line emission,
multi-token quoting, deterministic ordering) plus the empty-headers
no-regression case.
Compatibility:
- Zero existing SC consumers of `config.headers:` were found across the
codebases I have access to (grep across all PAY-SPACE stack yamls
returned no matches). The output for the empty-headers case is
byte-identical to before, so all existing stacks are unaffected.
Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Semgrep Scan ResultsRepository:
Scanned at 2026-05-29 09:17 UTC |
Security Scan ResultsRepository:
Scanned at 2026-05-29 09:17 UTC |
…pty case Addresses review findings from claude-opus-4-8 + codex: 1. `%q` vs Caddy's quoted-string lexer: agree on `\"` and `\\` (the only escape sequences both understand); diverge on `\n`/`\t`/control chars, which RFC 9110 forbids in HTTP header values anyway. Added a comment block explaining the constraint so a future reader knows the round-trip is intentional but bounded. 2. Missing test for the escape path itself — added TestCaddyfileEntry_HeaderValueWithEmbeddedDoubleQuote with a realistic CSP fragment carrying a literal `"` so a regression to bare `%s` would be caught. 3. Missing test for the Prefix template variant (Domain="", Prefix="api" → handle_path path). The same addHeaders placeholder feeds both template branches, but without coverage the second branch had zero CI signal. Added TestCaddyfileEntry_HeadersOnPrefixTemplate. 4. Empty-headers compatibility was only asserted via substring + absence, not byte-identical-to-pre-fix. Added TestCaddyfileEntry_EmptyHeadersByteIdenticalToPreFix locking the `header_down Server nginx \n` shape so the parent-Caddy change-hash for header-less stacks doesn't drift on the SC upgrade. 5. Reworded the determinism test comment to make clear the two-render-equal check is supplementary; the alphabetical-index assertion is the load-bearing proof. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
3-reviewer pass (claude-opus-4-8 + codex; gemini unavailable — API key expired)Verdict from both reviewers: no BLOCKER, ship-able. Same IMPORTANT items raised in both reviews — addressed in commit 16ee262. Addressed
Skipped (NIT, agreed-with rationale)
Test resultsAll 7 caddyfile tests pass: Broader |
End-to-end verification — PASSEDBuilt a branch preview and pinned a downstream consumer's deploy workflow to it on a throwaway test branch. Added three Pulumi diff from the deploy log (proves SC renders correctly)All four fix properties confirmed in the rendered annotation:
Live curl from the test host (proves Caddy actually emits them)Both custom headers land in the HTTP response after passing through Caddy. Ready to merge. |
Summary
The
args.Headersrendering path in the per-stack Caddyfile template has two coupled bugs that combine to makeconfig.headers:unusable from stack yaml. Surfaced when a consumer tried to ship a multi-header response baseline (XFO + CSP + Permissions-Policy etc.).Bugs
Bug 1 — same-line collapse
simple_container.go:685(and:696for the Prefix variant) contains:The first entry of
${addHeaders}rendered asheader_down X-...lands on the SAME LINE asheader_down Server nginx, producing:Caddy rejects line 1 with too-many-args → Caddyfile fails to load → deploy outage on first apply of any stack that sets
config.headers:.Bug 2 — unquoted values
Caddyfile is whitespace-tokenized; multi-word values (CSP, Permissions-Policy, multi-directive HSTS, anything with spaces or semicolons) decompose into extra arguments and crash the parser.
Fix
addHeaderswith a leading\nwhen non-empty so the first entry starts a new line.%qon the value — multi-token strings survive Caddy's tokenizer. Single-token values ("DENY") get harmless surrounding quotes, still valid Caddyfile.Tests
Four new regression tests; all four pass. Broader
pkg/clouds/pulumi/kubernetes/...suite stays green.Compatibility
A grep across the codebases I have access to found zero existing consumers of
config.headers:. The empty-headers output is byte-identical to before this fix, so all existing stacks are unaffected.